Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix some issues for windows #43

Merged
merged 1 commit into from
Jan 21, 2025
Merged

Fix some issues for windows #43

merged 1 commit into from
Jan 21, 2025
+2 −2

Conversation

Dcallies
Copy link
Contributor

@Dcallies Dcallies commented Jun 9, 2024

Hey, I was playing around with your library on my windows machine, and noticed the following:

  1. I get a decode error when using pip to install locally from the long_description trying to read the README. Switching to a simple encoding fixes it and allows local installs on windows, but probably breaks other things (guessing it's the emoji characters)
    1. I can try reading it first unencoded, and only falling back if it fails
  2. This one is weird - I assume a previous version of requests auto-set JSON content type or the Vestaboard API didn't enforce it, but it appears that it is enforcing now, or at least fails to post if JSON is not explicitly set.

Let me know what you want me to do #2, I also didn't spend any time looking for other sites that potentially needed the content type set explicitly.

@Dcallies
Copy link
Contributor Author

Dcallies commented Jun 9, 2024

Reading some more, it might be the difference between json= and data= on requests - See https://requests.readthedocs.io/en/latest/user/quickstart/#more-complicated-post-requests "Please note that the above code will NOT add the Content-Type header"

@ShaneSutro
Copy link
Owner

Hi @Dcallies!
Thanks for the PR, I'll review and get this merged! Looks good as far as I can tell, I've got something to fix on my deployment pipeline it looks like.

ShaneSutro
ShaneSutro approved these changes Jan 21, 2025
View reviewed changes
Copy link
Owner

@ShaneSutro ShaneSutro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for making these changes! Looks good from my end.

@ShaneSutro ShaneSutro merged commit c120ab1 into ShaneSutro:master Jan 21, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ShaneSutro ShaneSutro ShaneSutro approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Dcallies @ShaneSutro